Introduce prepare for eval, fix evaluation bug #789
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
submission_runner.py
Outdated
| @@ -518,6 +548,7 @@ def score_submission_on_workload(workload: spec.Workload, | |||
| init_optimizer_state = submission_module.init_optimizer_state | |||
| update_params = submission_module.update_params | |||
| data_selection = submission_module.data_selection | |||
| prepare_for_eval = submission_module.prepare_for_eval | |||
There was a problem hiding this comment.
This will break the submission modules of the competition. Should we default this to a no op function maybe in a try except block?
There was a problem hiding this comment.
It depends if we want to make the code backward compatible with the previous submissions. If so, we could make
prepare_for_eval a no-op by doing smth like this:
prepare_for_eval = getattr(submission_module, 'prepare_for_eval', None)Ant then call it only if it is not None:
# Prepare for evaluation (timed).
if prepare_for_eval is not None:
with profiler.profile('Prepare for eval'):
del batch
prepare_for_eval_start_time = get_time()
optimizer_state, model_params, model_state = prepare_for_eval(...)
prepare_for_eval_end_time = get_time()
# Update sumbission time.
train_state['accumulated_submission_time'] += (
prepare_for_eval_end_time - prepare_for_eval_start_time)What do you think?
My only doubt is that I am not sure that we can make all the API changes backward compatible to the submissions, for example #785 is more tricky to implement in such a way.
There was a problem hiding this comment.
At the moment, I think backwards compatibility is important since we are transitioning to the rolling leaderboard (which may involve hardware changes) and will have to potentially rescore some submissions.
I don't think we want to bear the responsibility to modify the existing submissions' code.
So I prefer this solution to get the prepare_for_eval function and call it only it if it is not None.
There was a problem hiding this comment.
I agree, it makes sense! I changed the code in this regard and tested it for backward compatibility, it looks good.
Description
This pull request introduces a
prepare_for_evalfunction and updates the code to support it.The implementation follows the blueprint of @fsschneider in #719 (comment) and fixes the bug of giving a free evaluation to a submission that goes out of max_runtime (again #719 (comment)).
Function signature
The arguments of
prepare_for_evalare the same asupdate_params, except forbatch. I believe thatprepare_for_evalshould indeed be agnostic to the last batch used during training. The return type is the same asupdate_params.List of changes
In
submission_runner.py:prepare_for_evalprofilerdel batchbeforeprepare_for_eval(instead than before evaluation)accumulated_submission_timeafterprepare_for_evalis_time_remainingafterprepare_for_evalis_time_remainingprep_eval_rngMinor changes:
PrepareForEvalFntospecprepare_for_evalto submission templateprepare_for_evalto all pytorch and jax submissionsFixes #719 and #758 .